Skip to content

Conversation

@titusfortner
Copy link
Member

@titusfortner titusfortner commented Jan 6, 2026

User description

💥 What does this PR do?

Run Java tests on RBE with chrome-beta

🔧 Implementation Notes

We added chrome beta a while back for Ruby, didn't add it in other places. Easy to wire it up in Java

💡 Additional Considerations

May still need to adjust which tests are run with which prod vs beta since everything on both is probably unnecessary


PR Type

Enhancement


Description

  • Add Chrome Beta support to Java test infrastructure

  • Define Chrome Beta JVM flags for driver and binary configuration

  • Register Chrome Beta as a new browser option in test configuration

  • Support Chrome Beta on Linux and macOS platforms


Diagram Walkthrough

flowchart LR
  A["Java Test Config"] -->|"adds Chrome Beta"| B["browsers.bzl"]
  B -->|"defines flags"| C["chromedriver_beta_jvm_flags"]
  B -->|"defines flags"| D["chrome_beta_jvm_flags"]
  E["selenium_test.bzl"] -->|"imports Beta data"| F["chrome_beta_data"]
  E -->|"imports Beta flags"| G["chrome_beta_jvm_flags"]
  E -->|"registers"| H["chrome-beta browser"]
Loading

File Walkthrough

Relevant files
Configuration changes
browsers.bzl
Define Chrome Beta JVM flags and configuration                     

java/browsers.bzl

  • Add chromedriver_beta_jvm_flags variable with Linux and macOS Chrome
    Beta driver locations
  • Add chrome_beta_jvm_flags variable combining Chrome Beta binary paths
    and driver flags
  • Configure headless mode support for Chrome Beta tests
+28/-0   
selenium_test.bzl
Register Chrome Beta as test browser option                           

java/private/selenium_test.bzl

  • Import chrome_beta_data and chrome_beta_jvm_flags from common and Java
    browser configs
  • Add new "chrome-beta" entry to BROWSERS dictionary with Chrome Beta
    dependencies and flags
  • Tag Chrome Beta tests with "chrome" and "chrome-beta" labels
+8/-0     

@selenium-ci selenium-ci added C-java Java Bindings B-build Includes scripting, bazel and CI integrations labels Jan 6, 2026
@qodo-code-review
Copy link
Contributor

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review
Copy link
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
fix local binary condition

In chrome_beta_jvm_flags, correct the condition key from
@selenium//common:use_local_chromedriver to @selenium//common:use_local_chrome
to properly handle local Chrome Beta binaries.

java/browsers.bzl [39-55]

 chrome_beta_jvm_flags = select({
     "@selenium//common:use_pinned_linux_chrome": [
         "-Dwebdriver.chrome.binary=$(location @linux_beta_chrome//:chrome-linux64/chrome)",
     ],
     "@selenium//common:use_pinned_macos_chrome": [
         "-Dwebdriver.chrome.binary=$(location @mac_beta_chrome//:Chrome.app)/Contents/MacOS/Chrome",
     ],
-    "@selenium//common:use_local_chromedriver": [],
+    "@selenium//common:use_local_chrome": [],
     "//conditions:default": [
         "-Dselenium.skiptest=false",
     ],
 }) + select({
     "@selenium//common:use_headless_browser": [
         "-Dwebdriver.headless=true",
     ],
     "//conditions:default": [],
 }) + chromedriver_beta_jvm_flags
  • Apply / Chat
Suggestion importance[1-10]: 9

__

Why: This suggestion correctly identifies a copy-paste error where the condition @selenium//common:use_local_chromedriver was used in a context that configures the Chrome binary, not the driver. Fixing this is critical for the local Chrome Beta configuration to work correctly.

High
Skip tests if browser is missing

In chrome_beta_jvm_flags, change the default behavior to skip tests by setting
selenium.skiptest to true to avoid failures when a Chrome Beta binary is not
available.

java/browsers.bzl [47-49]

 "//conditions:default": [
-    "-Dselenium.skiptest=false",
+    "-Dselenium.skiptest=true",
+    "-Dselenium.skiptest.reason='Only runs with pinned Chrome Beta or when local Chrome Beta is available'",
 ],
  • Apply / Chat
Suggestion importance[1-10]: 8

__

Why: This is a valid suggestion that improves the robustness of the new test configuration by preventing test failures in environments where Chrome Beta is not explicitly configured, which is a sensible default.

Medium
High-level
Refactor browser configuration to reduce duplication

To reduce duplication, refactor the browser configuration logic into a reusable
Starlark function that can generate configurations for different browser
channels (e.g., stable, beta) from parameters.

Examples:

java/private/selenium_test.bzl [23-34]
    "chrome": {
        "deps": ["//java/src/org/openqa/selenium/chrome"],
        "jvm_flags": ["-Dselenium.browser=chrome"] + chrome_jvm_flags,
        "data": chrome_data,
        "tags": COMMON_TAGS + ["chrome"],
    },
    "chrome-beta": {
        "deps": ["//java/src/org/openqa/selenium/chrome"],
        "jvm_flags": ["-Dselenium.browser=chrome"] + chrome_beta_jvm_flags,
        "data": chrome_beta_data,

 ... (clipped 2 lines)
java/browsers.bzl [11-55]
chromedriver_beta_jvm_flags = select({
    "@selenium//common:use_pinned_linux_chrome": [
        "-Dwebdriver.chrome.driver=$(location @linux_beta_chromedriver//:chromedriver)",
    ],
    "@selenium//common:use_pinned_macos_chrome": [
        "-Dwebdriver.chrome.driver=$(location @mac_beta_chromedriver//:chromedriver)",
    ],
    "//conditions:default": [],
})


 ... (clipped 35 lines)

Solution Walkthrough:

Before:

# java/browsers.bzl
chromedriver_jvm_flags = select({ ... paths for stable driver ... })
chrome_jvm_flags = select({ ... paths for stable binary ... }) + chromedriver_jvm_flags

chromedriver_beta_jvm_flags = select({ ... paths for beta driver ... })
chrome_beta_jvm_flags = select({ ... paths for beta binary ... }) + chromedriver_beta_jvm_flags

# java/private/selenium_test.bzl
BROWSERS = {
    "chrome": {
        "jvm_flags": ["-Dselenium.browser=chrome"] + chrome_jvm_flags,
        ...
    },
    "chrome-beta": {
        "jvm_flags": ["-Dselenium.browser=chrome"] + chrome_beta_jvm_flags,
        ...
    },
    ...
}

After:

# In a new function in a .bzl file
def create_browser_config(name, channel="stable"):
    # ... logic to generate jvm_flags based on name and channel ...
    return jvm_flags

def create_browser_entry(name, channel="stable"):
    key = name if channel == "stable" else name + "-" + channel
    return {
        key: {
            "jvm_flags": ["-Dselenium.browser=" + name] + create_browser_config(name, channel),
            # ... other common properties ...
        }
    }

# java/private/selenium_test.bzl
BROWSERS = {}
BROWSERS.update(create_browser_entry("chrome", "stable"))
BROWSERS.update(create_browser_entry("chrome", "beta"))
# ...
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies significant code duplication for browser configurations in java/browsers.bzl and java/private/selenium_test.bzl, proposing a valuable refactoring that would improve long-term maintainability.

Medium
General
support local Chromedriver

Add a @selenium//common:use_local_chromedriver condition to
chromedriver_beta_jvm_flags to support using a locally installed chromedriver
for beta tests.

java/browsers.bzl [11-19]

 chromedriver_beta_jvm_flags = select({
     "@selenium//common:use_pinned_linux_chrome": [
         "-Dwebdriver.chrome.driver=$(location @linux_beta_chromedriver//:chromedriver)",
     ],
     "@selenium//common:use_pinned_macos_chrome": [
         "-Dwebdriver.chrome.driver=$(location @mac_beta_chromedriver//:chromedriver)",
     ],
+    "@selenium//common:use_local_chromedriver": [],
     "//conditions:default": [],
 })
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: This suggestion correctly points out a missing configuration for using a local chromedriver with Chrome Beta, improving feature parity with the stable Chrome configuration and enhancing flexibility.

Medium
Learned
best practice
Deduplicate chrome browser definitions

The new chrome-beta entry duplicates most of chrome; factor out a small helper
(or shared base dict) so only the differing flags/data/tags are specified per
variant.

java/private/selenium_test.bzl [22-34]

+def _chrome_browser(jvm_flags, data, extra_tags = []):
+    return {
+        "deps": ["//java/src/org/openqa/selenium/chrome"],
+        "jvm_flags": ["-Dselenium.browser=chrome"] + jvm_flags,
+        "data": data,
+        "tags": COMMON_TAGS + ["chrome"] + extra_tags,
+    }
+
 BROWSERS = {
-    "chrome": {
-        "deps": ["//java/src/org/openqa/selenium/chrome"],
-        "jvm_flags": ["-Dselenium.browser=chrome"] + chrome_jvm_flags,
-        "data": chrome_data,
-        "tags": COMMON_TAGS + ["chrome"],
-    },
-    "chrome-beta": {
-        "deps": ["//java/src/org/openqa/selenium/chrome"],
-        "jvm_flags": ["-Dselenium.browser=chrome"] + chrome_beta_jvm_flags,
-        "data": chrome_beta_data,
-        "tags": COMMON_TAGS + ["chrome", "chrome-beta"],
-    },
+    "chrome": _chrome_browser(chrome_jvm_flags, chrome_data),
+    "chrome-beta": _chrome_browser(chrome_beta_jvm_flags, chrome_beta_data, ["chrome-beta"]),

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 6

__

Why:
Relevant best practice - Reduce duplication by centralizing shared behavior (avoid near-identical map entries for related browsers).

Low
Centralize duplicated headless flags

The headless select(...) is duplicated; extract it into a shared
headless_jvm_flags variable (and reuse it for both chrome and chrome-beta) to
keep the logic single-sourced.

java/browsers.bzl [39-55]

+headless_jvm_flags = select({
+    "@selenium//common:use_headless_browser": [
+        "-Dwebdriver.headless=true",
+    ],
+    "//conditions:default": [],
+})
+
 chrome_beta_jvm_flags = select({
     "@selenium//common:use_pinned_linux_chrome": [
         "-Dwebdriver.chrome.binary=$(location @linux_beta_chrome//:chrome-linux64/chrome)",
     ],
     "@selenium//common:use_pinned_macos_chrome": [
         "-Dwebdriver.chrome.binary=$(location @mac_beta_chrome//:Chrome.app)/Contents/MacOS/Chrome",
     ],
     "@selenium//common:use_local_chromedriver": [],
     "//conditions:default": [
         "-Dselenium.skiptest=false",
     ],
-}) + select({
-    "@selenium//common:use_headless_browser": [
-        "-Dwebdriver.headless=true",
-    ],
-    "//conditions:default": [],
-}) + chromedriver_beta_jvm_flags
+}) + headless_jvm_flags + chromedriver_beta_jvm_flags
  • Apply / Chat
Suggestion importance[1-10]: 5

__

Why:
Relevant best practice - Reduce duplication by centralizing shared behavior (avoid repeating identical select blocks across new/related configs).

Low
  • More

@titusfortner titusfortner merged commit f65d311 into trunk Jan 8, 2026
23 checks passed
@titusfortner titusfortner deleted the java_chrome_beta branch January 8, 2026 18:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

B-build Includes scripting, bazel and CI integrations C-java Java Bindings Review effort 2/5

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants